Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

libxls: add recipe #12152

Merged
merged 24 commits into from
Jan 27, 2023
Merged

libxls: add recipe #12152

merged 24 commits into from
Jan 27, 2023

Conversation

toge
Copy link
Contributor

@toge toge commented Aug 11, 2022

Specify library name and version: libxls/1.6.2

libxls is xls(old type Excel) file reader library.

  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the conan-center hook activated.

@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@github-actions github-actions bot mentioned this pull request Aug 12, 2022
Open
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
uilianries
uilianries previously approved these changes Aug 12, 2022
View reviewed changes
@conan-center-bot

This comment has been minimized.

Sign in to view
@stale stale bot removed the stale label Oct 17, 2022
@conan-center-bot

This comment has been minimized.

Sign in to view
@toge
Copy link
Contributor Author

toge commented Oct 17, 2022

@uilianries
Now cross compiling for M1 macOS works fine.
However, compiling with MSVC fails.

I think I can compile it because upstream has an issue on MSVC, but I couldn't figure out how to fix it on AutotoolsToolchain.
Do you know how to compile with MSVC in AutotoolsToolchain?
If there is no way, I would like to remove MSVC support in validate().

@uilianries
Copy link
Member

@toge Yes, we have few good examples, but they work, as this one:https://github.com/conan-io/conan-center-index/pull/13263/files

Basically, you need to get Environment object from AutotoolsToochains, set CC=cl.exe and others.

Another good option and more clear (Which I prefer): https://github.com/conan-io/conan-center-index/blob/master/recipes/libbacktrace/all/conanfile.py#L91

In case it does not work, please, skip it.

@toge
Copy link
Contributor Author

toge commented Oct 17, 2022

@uilianries
Thanks a lot!
I try them.

@toge toge marked this pull request as draft October 18, 2022 12:25
@stale
Copy link

stale bot commented Nov 19, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 19, 2022
@stale stale bot removed the stale label Dec 12, 2022
@conan-center-bot

This comment has been minimized.

Sign in to view
@toge toge marked this pull request as ready for review December 12, 2022 17:05
@toge
Copy link
Contributor Author

toge commented Dec 12, 2022

I have given up support for Windows...

prince-chrismc
prince-chrismc reviewed Dec 14, 2022
View reviewed changes
Copy link
Contributor

@prince-chrismc prince-chrismc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As always your work is out standinging.

I just had one question and a few linter things I pointed out :)

recipes/libxls/all/conanfile.py Outdated Show resolved Hide resolved
recipes/libxls/all/conanfile.py Outdated Show resolved Hide resolved
recipes/libxls/all/conanfile.py Outdated Show resolved Hide resolved
recipes/libxls/all/conanfile.py Outdated Show resolved Hide resolved
recipes/libxls/all/conanfile.py
Comment on lines +104 to +105
if is_apple_os(self):
self.cpp_info.system_libs.append("iconv")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am surprised it's a system lib on Mac 🤔 not usually the case... care to share with me? I am not familar with this project

Suggested change
if is_apple_os(self):
self.cpp_info.system_libs.append("iconv")
if is_apple_os(self):
self.cpp_info.system_libs.append("iconv")
else:
self.cpp_info.requires.append("libiconv::libiconv")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@prince-chrismc
Sorry, I neglected it so much that I forgot some of it.
I will investigate again.

recipes/libxls/all/conanfile.py
Comment on lines +111 to +114
if not is_apple_os(self):
self.cpp_info.requires.append("libiconv::libiconv")

# TODO: Remove in Conan 2.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if not is_apple_os(self):
self.cpp_info.requires.append("libiconv::libiconv")
# TODO: Remove in Conan 2.0
# TODO: Remove in Conan 2.0

toge and others added 4 commits December 15, 2022 14:42
@toge @prince-chrismc
removed unused import
10cd02f 
Co-authored-by: Chris Mc <prince.chrismc@gmail.com>
@toge @prince-chrismc
remove replace_in_file
f74c299 
Co-authored-by: Chris Mc <prince.chrismc@gmail.com>
@toge @prince-chrismc
remove unused import
aa8b44e 
Co-authored-by: Chris Mc <prince.chrismc@gmail.com>
@toge @prince-chrismc
improve validation error message
8c75cf4 
Co-authored-by: Chris Mc <prince.chrismc@gmail.com>
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline

All green in build 20 (8c75cf460b70e57be89d1a424c3fd5c9dbbcc205):

  • libxls/1.6.2@:
    All packages built successfully! (All logs)

@toge toge marked this pull request as draft December 15, 2022 08:20
@prince-chrismc
Copy link
Contributor

CI is happy and this a a lot of great work, I'd love to see it go in

@toge toge marked this pull request as ready for review January 11, 2023 08:51
uilianries
uilianries approved these changes Jan 27, 2023
View reviewed changes
Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@conan-center-bot conan-center-bot merged commit ec7a07a into conan-io:master Jan 27, 2023
StellaSmith pushed a commit to StellaSmith/conan-center-index that referenced this pull request Feb 2, 2023
@toge @uilianries
@prince-chrismc @StellaSmith
(conan-io#12152) libxls: add recipe
6081fed 
* libxls: add recipe

* fix KB of cci

* skip pylint in test_V1_package

* set HAVE_XLOCALE

* link libiconv

* don't link iconv on macos

* link iconv in CMakeLists.txt

* fix package name

* support ssize_t in MSVC

* follow conan v2

* fix self.options.shared wrong usage

* replace restrict for msvc 15

* use autotools

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* not autoreconf

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* Windows build

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* license

Signed-off-by: Uilian Ries <uilianries@gmail.com>

* update conan 1.52.0

* fix rpl_malloc undefined eerror

* drop support windows

* removed unused import

Co-authored-by: Chris Mc <prince.chrismc@gmail.com>

* remove replace_in_file

Co-authored-by: Chris Mc <prince.chrismc@gmail.com>

* remove unused import

Co-authored-by: Chris Mc <prince.chrismc@gmail.com>

* improve validation error message

Co-authored-by: Chris Mc <prince.chrismc@gmail.com>

Signed-off-by: Uilian Ries <uilianries@gmail.com>
Co-authored-by: Uilian Ries <uilianries@gmail.com>
Co-authored-by: Chris Mc <prince.chrismc@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prince-chrismc prince-chrismc prince-chrismc approved these changes

@uilianries uilianries uilianries approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@toge @conan-center-bot @uilianries @prince-chrismc